fix(security): remove shell: true from execa calls to prevent command injection - #449
Conversation
… injection - sandbox-tester.js: remove redundant shell: true (args already split) - subtask-verifier.js: remove shell: true from execa path - subtask-verifier.js: replace execSync fallback with execFileSync + arg splitting - Both scripts already split commands into program + args array, making shell interpretation unnecessary and a security risk (metacharacter injection) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@riaworks is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@coderabbitai ignore |
|
✅ Actions performedReviews paused. |
Summary
shell: truefromexeca()inrunCommand()— args were already split into array, making shell interpretation unnecessary and a security riskshell: truefromexeca()path and replaceexecSyncfallback withexecFileSync+ proper argument splittingContext
Both scripts already split command strings into
program + argsarray before passing toexeca(), makingshell: truecompletely redundant. Withshell: true, shell metacharacters (;,|,&&,$()) in command inputs would be interpreted, enabling potential command injection.Risk Assessment
sandbox-tester.jswas flagged as the most dangerous file in the codebase due toshell: true+ arbitrary command executionsubtask-verifier.jscombines shell execution + network access + browser automation — broad attack surfaceChanges
sandbox-tester.js:454shell: truefromexeca()optionssubtask-verifier.js:277execSyncwithexecFileSync+ arg splittingsubtask-verifier.js:304shell: truefromexeca()optionsTest plan
npm testpassesnpm testandnpm run lintcommands correctly withoutshell: trueexecFileSyncfallback🤖 Generated with Claude Code